Skip to content

Add DENM TS access functions#114

Open
jdvoliveira wants to merge 6 commits intoika-rwth-aachen:mainfrom
es-av-it-pt:denm-ts-access-functions
Open

Add DENM TS access functions#114
jdvoliveira wants to merge 6 commits intoika-rwth-aachen:mainfrom
es-av-it-pt:denm-ts-access-functions

Conversation

@jdvoliveira
Copy link
Contributor

This draft PR adds the initial headers for the denm_ts access functions.
The function implementations are still pending and will be added gradually.

@jdvoliveira jdvoliveira changed the title Add DENM TS access functions WIP: Add DENM TS access functions Jan 26, 2026
@jdvoliveira jdvoliveira closed this Feb 2, 2026
@jdvoliveira jdvoliveira deleted the denm-ts-access-functions branch February 2, 2026 15:23
@jdvoliveira jdvoliveira restored the denm-ts-access-functions branch February 2, 2026 15:24
@jdvoliveira jdvoliveira reopened this Feb 2, 2026
@jdvoliveira jdvoliveira changed the title WIP: Add DENM TS access functions Add DENM TS access functions Feb 2, 2026
@jdvoliveira jdvoliveira marked this pull request as ready for review February 2, 2026 15:31
@jdvoliveira
Copy link
Contributor Author

I’ve now added the denm_ts access function implementations.
Feedback is welcome.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds initial support for DENM TS (Technical Specification) access functions to the ETSI ITS messages utilities library. The PR introduces new header files for DENM TS message manipulation alongside the existing DENM EN (European Norm) support, with significant code refactoring to share common functionality between the two variants.

Changes:

  • Added DENM TS access headers and implementation files (denm_ts_access.hpp, denm_ts_access.h, denm_ts_getters.h, denm_ts_setters.h)
  • Refactored common DENM functionality into shared headers (denm_getters_common.h, denm_setters_common.h)
  • Added CDD v2.2.1 support (cdd_v2-2-1_getters.h, cdd_v2-2-1_setters.h)
  • Added corresponding test file (test_denm_ts_access.cpp)
  • Updated denm_utils.h to support both EN and TS variants

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
etsi_its_msgs_utils/test/test_access.cpp Added includes and namespace setup for DENM TS test integration
etsi_its_msgs_utils/test/impl/test_denm_ts_access.cpp New test file for DENM TS access functions with comprehensive test coverage
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_utils.h Updated to support both EN and TS, added header guards (with ordering issue)
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_ts_setters.h New TS-specific setter functions for DENM
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_ts_getters.h New TS-specific getter functions including extensive cause code mappings
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_ts_access.h Main DENM TS access header with namespace setup (missing v2.2.1 guards)
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_setters_common.h Extracted common setter functions shared by EN and TS
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_setters.h Refactored to use common setters, removed duplicated code
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_getters_common.h Extracted common getter functions shared by EN and TS
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_getters.h Refactored to use common getters, removed duplicated code
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_access.h Updated to include new common header guards
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/cdd/cdd_v2-2-1_setters.h New CDD v2.2.1 setter functions including WGS84 heading support
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/cdd/cdd_v2-2-1_getters.h New CDD v2.2.1 getter functions for WGS84 heading
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/cdd/cdd_v2-1-1_setters.h Refactored to move common functions to cdd_setters_common.h
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/cdd/cdd_v1-3-1_setters.h Refactored to move common functions to cdd_setters_common.h
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/cdd/cdd_setters_common.h Added common CDD setter functions (setStationId, setStationType) as templates
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/cdd/cdd_getters_common.h Updated description to include v2.2.1
etsi_its_msgs_utils/include/etsi_its_msgs_utils/denm_ts_access.hpp Public header for DENM TS access in ROS 2 projects

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jdvoliveira
Copy link
Contributor Author

All comments have been addressed.
Please let me know if anything else needs adjustment.

@gkueppers gkueppers self-requested a review March 6, 2026 12:58
* @return WGS Heading value in degree as decimal number
*/
template <typename Wgs84Angle>
inline double getWGSHeadingCDD(const Wgs84Angle& heading) { return ((double)heading.value.value) * 1e-1; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a specific reason for this CDD postfix in the function-name? I think we're not having these for the ohter setter/getter functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CDD postfix is already used for low-level CDD functions (e.g. getHeadingCDD(), setHeadingCDD(), getYawRateCDD() in cdd_*_common.h). The message-level access functions stay without postfix. getWGSHeadingCDD() follows the same pattern.

*/

template <typename Wgs84Angle>
inline double getWGSHeadingConfidenceCDD(const Wgs84Angle& heading) { return ((double)heading.confidence.value) * 1e-1 / etsi_its_msgs::ONE_D_GAUSSIAN_FACTOR; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: CDD postfix?

* @param confidence standard deviation of heading in degree as decimal number (default: infinity, mapping to Wgs84AngleConfidence::UNAVAILABLE)
*/
template <typename Wgs84Angle, typename Wgs84AngleConfidence = decltype(Wgs84Angle::confidence)>
void setWGSHeadingCDD(Wgs84Angle& heading, const double value, double confidence = std::numeric_limits<double>::infinity()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: CDD postfix?

Comment on lines -38 to -39
#undef ETSI_ITS_MSGS_UTILS_IMPL_CDD_CDD_V2_1_1_GETTERS_H
#undef ETSI_ITS_MSGS_UTILS_IMPL_CDD_CDD_V2_1_1_SETTERS_H
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To my understanding DENM EN uses CDD v1.3.1 and DENM TS (Release 2) is based on CDD v2.2.1 so why are we replacing the undef of v2.1.1 with v2.2.1? I guess we should remove the "undef" for v.1.3.1 and keep both undefs vor v2.1.X instead?!
At least this should be aligned to what happens in denm_ts_access.h

Copy link
Contributor Author

@jdvoliveira jdvoliveira Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I’ve updated the undefs in both denm_acess.h and denm_ts_access.h.

* @return WGS Heading value in degree as decimal number
*/
template <typename Wgs84Angle>
inline double getWGSHeadingCDD(const Wgs84Angle& heading) { return ((double)heading.value.value) * 1e-1; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a specific reason for this CDD postfix? I think we're not having these for the

#undef ETSI_ITS_MSGS_UTILS_IMPL_CDD_CDD_V1_3_1_GETTERS_H
#undef ETSI_ITS_MSGS_UTILS_IMPL_CDD_CDD_V1_3_1_SETTERS_H
#undef ETSI_ITS_MSGS_UTILS_IMPL_CDD_CDD_V2_2_1_GETTERS_H
#undef ETSI_ITS_MSGS_UTILS_IMPL_CDD_CDD_V2_2_1_SETTERS_H
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we undef v1.3.1 and v2.2.1 because DENM TS is based on v2.1.1 so in denm_access.h we should undef v2.1.1 and v2.2.1 based on my understanding

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DENM TS is based on v2.2.1 so I’ve updated this and now we undef v1.3.1 and v2.1.1.

* @param denm DENM to get the causeCode value from
* @return causeCode value
*/
inline uint8_t getCauseCodeV2(const DENM& denm) { return denm.denm.situation.event_type.cc_and_scc.choice; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as with the CDD postfix: Do we really need this postfix? Shouldn't this be solved by the different namespaces etsi_its_denm_msgs::access for release 1 and etsi_its_denm_ts_msgs::access for release 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially used the V2 suffix because the field type is named CauseCodeV2, but you are correct that the namespaces already distinguish EN vs TS. I've renamed these functions to keep the naming consistent.

@@ -0,0 +1,77 @@
#include <gtest/gtest.h>
#include <cmath>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're also lacking this tests for DENM EN but we might add some exemplary for the (Sub)CauseCode setters/getters

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I added some exemplary tests for the (Sub)CauseCode functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants